Skip to content

Comments

Proposed patch for issue #11#13

Merged
StanleySathler merged 2 commits intoss-javascript:masterfrom
coderwassananmol:patch-1
Oct 8, 2017
Merged

Proposed patch for issue #11#13
StanleySathler merged 2 commits intoss-javascript:masterfrom
coderwassananmol:patch-1

Conversation

@coderwassananmol
Copy link
Contributor

Patch for #11

@StanleySathler
Copy link
Member

Hello @coderwassananmol
Great commit, bro!

Can I give you a better approach?

As explained here, the problem occurs because the part of the code which calculates the content's height is not considering the presence of the filter.

Check the file _popup.js, line 170. There, you can see that the content's height is calculated based on popup's height and the title's height.

If the select has a filter (remember: there is a parameter saying that, default is false), we should consider the filter too. The solution would be something like that:

let popupHeight = _calculatePopupHeight();
let titleHeight = _$title.outerHeight();
let filterHeight = _$filter.outerHeight();

let contentHeight = (popupHeight - titleHeight);
if(_properties.hasFilter)
	contentHeight = contentHeight - filterHeight;

_$content.outerHeight(contentHeight);

You must check if the hasFilter is true because even when the filter field is not visible, it has the size 44px. It's because its visibility is controlled by display: none and not something like height: 0

@coderwassananmol
Copy link
Contributor Author

Brilliant @StanleySathler
I have updated the patch. I hope this works!

@StanleySathler StanleySathler merged commit 5b3b6c2 into ss-javascript:master Oct 8, 2017
@StanleySathler
Copy link
Member

Yay! Very excited with the contribution, @coderwassananmol. Already merged. 🔥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants